Skip to content

Comments

Implement onFileDeleted->onFileDelete method name change#1517

Merged
niveathika merged 9 commits intoballerina-platform:masterfrom
niveathika:master
Dec 16, 2025
Merged

Implement onFileDeleted->onFileDelete method name change#1517
niveathika merged 9 commits intoballerina-platform:masterfrom
niveathika:master

Conversation

@niveathika
Copy link
Contributor

Purpose

Examples

Checklist

  • Linked to an issue
  • Updated the changelog
  • Added tests
  • Updated the spec
  • Checked native-image compatibility

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR implements a method name change from onFileDeleted to onFileDelete for handling file deletion events in the FTP service. The change includes deprecating the old method while maintaining backward compatibility, and modifies the callback behavior to invoke the handler once per deleted file (with a string parameter) instead of once for all files (with a string[] parameter).

Key Changes:

  • Introduced onFileDelete(string deletedFile) as the new deletion handler, called once per deleted file
  • Deprecated onFileDeleted(string[] deletedFiles) with warning diagnostics while maintaining backward compatibility
  • Updated compiler plugin validation to support both methods with appropriate deprecation warnings and mutual exclusivity checks

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
native/src/main/java/io/ballerina/stdlib/ftp/util/FtpConstants.java Added constant for new onFileDelete method name
native/src/main/java/io/ballerina/stdlib/ftp/util/FtpUtil.java Updated method detection to support both old and new method names
native/src/main/java/io/ballerina/stdlib/ftp/server/FtpListener.java Implemented new processDeletionCallback that dispatches to appropriate handler based on parameter type, added processFileDeleteCallback for per-file invocation
compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/PluginConstants.java Added new error codes and messages for onFileDelete validation and deprecation warnings
compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidator.java Added validation logic to check for both methods and ensure they're not used simultaneously
compiler-plugin/src/main/java/io/ballerina/stdlib/ftp/plugin/FtpFileDeletedValidator.java Refactored validator to support both method types with appropriate parameter validation
compiler-plugin-tests/src/test/java/io/ballerina/stdlib/ftp/plugin/FtpServiceValidationTest.java Added comprehensive tests for onFileDelete validation scenarios
compiler-plugin-tests/src/test/resources/ballerina_sources/* Added test cases for valid and invalid onFileDelete usage patterns
docs/spec/spec.md Updated specification to document the new onFileDelete method
ballerina/tests/listener_on_file_deleted_test.bal Updated tests to use new onFileDelete method with per-file invocation pattern
ballerina/client_endpoint.bal Repositioned deprecation notices (though placement is inconsistent with best practices)
ballerina/caller.bal Repositioned deprecation notices (though placement is inconsistent with best practices)
ballerina/Ballerina.toml Added unrelated ballerina/cluster dependency that should be justified or removed
changelog.md Documented the deprecation (with a spelling error)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov
Copy link

codecov bot commented Dec 9, 2025

Codecov Report

❌ Patch coverage is 89.58333% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.75%. Comparing base (d3b14c2) to head (621e44c).
⚠️ Report is 10 commits behind head on master.

Files with missing lines Patch % Lines
...va/io/ballerina/stdlib/ftp/server/FtpListener.java 75.67% 5 Missing and 4 partials ⚠️
...llerina/stdlib/ftp/plugin/FtpServiceValidator.java 92.85% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1517      +/-   ##
============================================
+ Coverage     80.41%   80.75%   +0.33%     
- Complexity      532      747     +215     
============================================
  Files            43       58      +15     
  Lines          2732     3357     +625     
  Branches        432      567     +135     
============================================
+ Hits           2197     2711     +514     
- Misses          357      404      +47     
- Partials        178      242      +64     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@niveathika niveathika added the Skip GraalVM Check This will skip the GraalVM compatibility check label Dec 9, 2025
@sonarqubecloud
Copy link

@niveathika niveathika merged commit 08eabac into ballerina-platform:master Dec 16, 2025
9 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Skip GraalVM Check This will skip the GraalVM compatibility check

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants